Skip to content

London | 26-ITP-Sep | Chandramani Gaire | Sprint 2 | Coursework/sprint 3 implement and rewrite - #1682

Open
gaireprakash20-ops wants to merge 22 commits into
CodeYourFuture:mainfrom
gaireprakash20-ops:coursework/sprint-3-implement-and-rewrite
Open

gaireprakash20-ops wants to merge 22 commits into
CodeYourFuture:mainfrom
gaireprakash20-ops:coursework/sprint-3-implement-and-rewrite

Conversation

@gaireprakash20-ops

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Task code

CYF-1059

Changelist

I worked as per the module instructions and implemented and tested the code.

Questions

I was a bit confused about whether I should delete the file where I spent my initial learning time with JavaScript.

@gaireprakash20-ops gaireprakash20-ops added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 11, 2026

@gaireprakash20-ops gaireprakash20-ops left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need reviews

@gaireprakash20-ops gaireprakash20-ops left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please review

@gaireprakash20-ops gaireprakash20-ops left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need review

@hey-hammad hey-hammad added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 15, 2026

@hey-hammad hey-hammad left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start, however please review the test cases in rewrite-tests-with-jest directory again some of them are failing. Thanks

return false;
}

if (numerator < 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job handling the negative values, however is there an easy way of handling negative values?

Think about which built-in function can be used for this scenario.


if (value === "A") {
return 11;
} else if (value === "J") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of checking each of the values between J, Q & K in separate else if statements, would it be better to do it in a single statement since all of them return the same result?

@hey-hammad hey-hammad added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 15, 2026
@gaireprakash20-ops gaireprakash20-ops added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 17, 2026
@hey-hammad hey-hammad added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants